Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set C++ version for boost thread #137

Merged
merged 1 commit into from
Mar 11, 2024
Merged

Set C++ version for boost thread #137

merged 1 commit into from
Mar 11, 2024

Conversation

aWZHY0yQH81uOYvH
Copy link
Contributor

@aWZHY0yQH81uOYvH aWZHY0yQH81uOYvH commented Mar 5, 2024 •
edited
Loading

Fixes thliebig/openEMS-Project#190

As of recent, nf2ff has been failing to compile with the latest version of boost (1.84.0):

In file included from /tmp/openEMS-Project/openEMS/nf2ff/nf2ff.cpp:19:
In file included from /tmp/openEMS-Project/openEMS/nf2ff/nf2ff_calc.h:26:
In file included from /usr/local/include/boost/thread.hpp:24:
In file included from /usr/local/include/boost/thread/future.hpp:31:
In file included from /usr/local/include/boost/thread/detail/invoker.hpp:32:
/usr/local/include/boost/thread/csbl/tuple.hpp:19:18: error: no member named 'tuple' in namespace 'std'
    using ::std::tuple;
          ~~~~~~~^
/usr/local/include/boost/thread/csbl/tuple.hpp:21:18: error: no member named 'make_tuple' in namespace 'std'
    using ::std::make_tuple;
          ~~~~~~~^

They changed around some stuff in this header file.

Setting the C++ version to 11 fixes this for me on

% clang --version
Apple clang version 15.0.0 (clang-1500.1.0.2.5)
Target: x86_64-apple-darwin23.3.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

@aWZHY0yQH81uOYvH aWZHY0yQH81uOYvH mentioned this pull request Mar 9, 2024
Closed
@thliebig
Copy link
Owner

Hmm, I'm always a but hesitant with such changes as people may want to compile openEMS on e.g. very old machines with potentially an even older compiler?
And on my quite up to date gcc I do not see this issue either.
Maybe we should put this behind a compiler switch (only for clang) or only for mac?? At least for the time being?
Opinions?

@aWZHY0yQH81uOYvH
Copy link
Contributor Author

On my install of GCC 13, it looks like the default __cplusplus version without any --std argument supplied is C++ 17, whereas on clang it's C++ 98. Specifying --std=c++98 for GCC makes it spit out even more errors than clang when dealing with the same boost/thread.hpp header.

It's mainly a problem with the combination of the old C++ standard and the new version of Boost, which uses C++ 11 features. Presumably an old machine that doesn't have a C++ 11 compiler wouldn't also be using a new version of Boost that requires a C++ 11 compiler.

I suppose the inclusion of this CMake line could be made dependent on the detected Boost version rather than the host platform or compiler? But honestly, I doubt there are many machines out there worth running openEMS on that don't have a C++ 11 compiler.

@aWZHY0yQH81uOYvH
Copy link
Contributor Author

Another consideration would be if this change (whatever its final form may be) should be moved elsewhere in the project, since nf2ff isn't the only thing using Boost. This specific header file is also used in the openEMS multithread engine as well.

@aWZHY0yQH81uOYvH
Copy link
Contributor Author

It looks like setting the C++ version to something the current compiler doesn't support is OK. According to the CMake docs,

If [CXX_STANDARD_REQUIRED] is OFF or unset, the CXX_STANDARD target property is treated as optional and may "decay" to a previous standard if the requested is not available.

So this shouldn't break the compilation with an older compiler as long as whatever dependency libraries are OK with the older C++ standard.

@thliebig
Copy link
Owner

Thanks for the analysis. Let's just apply this and cross our fingers that nothing breaks 😉

@thliebig thliebig merged commit 1ccf094 into thliebig:master Mar 11, 2024
@aWZHY0yQH81uOYvH
Copy link
Contributor Author

Sounds good 🙂

Looks like something somewhere is setting C++ 11 for the main openEMS build, which is why this change is only needed for nf2ff, so It's Probably Fine™

@aWZHY0yQH81uOYvH aWZHY0yQH81uOYvH deleted the fix-boost-thread branch March 11, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenEMS fails to build on Mac OS 14.1.2
2 participants
@aWZHY0yQH81uOYvH @thliebig